Skip to content

Tell the server which run of a browser the shared computer is on - #265

Open
beardthelion wants to merge 3 commits into
CopilotKit:mainfrom
beardthelion:fix/shared-computer-run-identity
Open

Tell the server which run of a browser the shared computer is on#265
beardthelion wants to merge 3 commits into
CopilotKit:mainfrom
beardthelion:fix/shared-computer-run-identity

Conversation

@beardthelion

Copy link
Copy Markdown
Contributor

What this changes

Ordering snapshots on (session, generation) landed in #173, and it is keyed on
ComputerProvider.sessionOf, which is optional. The Docker supervisor reads the run off the
container and the sandbox reads it off the moment its browser last became ready.
createSharedComputerProvider implements neither, so on the deployment with one computer for every
Bot the run was undefined on every save and every resolve, and both guards did nothing: the table's
setWhere fell back to comparing generations, and resolve short-circuited before comparing runs at
all.

Both failures #158 described are still reachable there, on main at e8aa344:

  • a save still in flight when a reset lands finds no row to conflict with, inserts the wiped page
    back, and every ref on it goes on resolving, so the boundary decides about a page that was deleted
  • a computer that restarts counts generations from one again, so its first snapshot reads as older
    than the dead page and is dropped until the counter climbs back past it

That process serves every Bot and outlives every reset, so nothing outside it can tell one browser
session from the next. The session mints a run, the reset handler mints a new one, and a read on the
computer answers which run a Bot is on. The sessions move to their own module because the rules about
when a run changes are worth testing and index.ts launches a browser the moment it is imported,
which is the same reason browser-eviction.ts and profile-listing.ts are separate files.

The run is also read before /snapshot rather than after it. Asked afterwards, a computer replaced
mid-snapshot has its dead page stamped with the run of the browser that replaced it, which is the
same pair of failures inside a smaller window.

Nothing changes for a deployment that already reports a run, and a computer too old to answer leaves
the ordering exactly where it was. Unknown is not mismatched: a fix that turned a missing answer into
a refusal would take a working deployment down on the way to fixing this.

This is the residual on #158, which was closed as fixed. It is fixed for the supervisor and sandbox
providers; this is the third one.

Where it runs

  • New state that outlives a request? The run, held per Bot in the computer's session map,
    which is where the generation counter it qualifies already lives. It is deliberately not shared
    state: the map belongs to the one process the shared deployment runs, and the answer is fetched
    over HTTP by whichever server replica needs it rather than replicated. On the server side the
    run goes in the session column Stop a ref resolving against a dead computer session #173 added, so it crosses replicas in the row it qualifies.
  • What happens on the second replica? It asks the computer, gets the same answer, and
    resolves against the same row. The run is deliberately not cached in the provider: the
    supervisor caches because locate is an /ensure that refreshes it before every action, while
    here locate is a string and makes no call, so a cached run would be the one that replica first
    saw for the life of the process.
  • Anything serialised? The conditional update that was already there. The run widens
    setWhere rather than adding a second write, so two replicas snapshotting one computer are
    still settled by Postgres in one statement, and ordering within a run is unchanged.
  • Anything fanned out to a browser? None.
  • New listener, port, or schedule? None. GET /run is a route on the port the computer
    already serves, behind the same COMPUTER_TOKEN as everything except /health. It is
    deliberately not on the acting list, because the server asks it on the path of every governed
    action including the ones it is about to refuse, and a 409 there would turn every ref the server
    holds into an unanswerable question for as long as somebody held the wheel.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act. The change
    is what resolve is given, not where the decision happens.
  • New refusals and new failures each write a row. A citation refused for naming a dead run takes
    the path that already existed and records element: "not in the current snapshot" rather than
    the dead page's button, which is what it recorded before.
  • Nothing new is trusted from the client. The run comes from the computer, asked by the server,
    on the same call path as the snapshot it qualifies.

Changelog

  • A line under Unreleased.

Proof

Reproduced first, on a branch off e8aa344, against a real Postgres with 0014 applied. Case A
resolved the wiped page's Confirm transfer with no StaleSnapshotError; case B kept generation 7
in both stores while generation 1 was dropped. The control, the same sequence with a run reported,
landed generation 1. The prediction that separates this from every other explanation: case A with a
provider that does report a run is refused, same store, same gateway, changing only whether
sessionOf answers.

Then 22 tests across four files, each observed red before the fix and green after:

  • agent-computer/tests/sessions.test.ts, when a run changes and when it must not, including a
    session the idle sweep dropped and the control that a running Bot keeps its run
  • agent-computer/tests/reset-run.test.ts, the reset handler driven over HTTP against the real
    process, because a unit test of the function that changes the run stays green if the handler never
    calls it. Deleting that one line turns exactly one of these red and leaves the other seven green.
    It never launches Chromium: neither endpoint asks for a page
  • server/tests/computer-provider.test.ts, the provider reporting the run per Bot, asking again
    rather than remembering, and answering undefined rather than throwing when the computer cannot say
  • server/tests/computer-gateway.test.ts and
    server/tests/shared-computer-run.integration.test.ts, both halves end to end through the real
    provider, against the in-memory store and against a real table, since ordering across runs is a
    comparison in TypeScript in one and a setWhere Postgres evaluates in the other. Removing
    sessionOf turns 3 of the 5 table-backed cases red and leaves the two controls green

Full suite 1734 pass, 0 fail. bun run typecheck clean, and agent-computer typechecked separately
with bunx tsc because the root filter does not reach it.

One thing not run: the two real processes against each other, and real Chromium. That needs Docker,
which I cannot reach here. The wire contract is pinned from both sides but never observed in one
process pair.

Worth flagging

ComputerProvider.sessionOf is still optional, which is the shape that caused this: an absent method
silently disables a boundary rather than failing to compile. Making it required is a small change
plus mechanical edits to the provider literals in five test files, and it seemed wider than this fix
should reach. Happy to do it here or separately if you want it.

The three comments that described the old behaviour are corrected, including the two flagged in the
#158 thread.

Ordering snapshots on (session, generation) is keyed on
ComputerProvider.sessionOf, which is optional. A Bot with its own
container reads the run off the container and a sandbox reads it off the
moment its browser last became ready, but createSharedComputerProvider
implements neither, so on the deployment with one computer for every Bot
the run was undefined on every save and every resolve. Both guards then
did nothing: the table's setWhere fell back to comparing generations, and
resolve short-circuited before comparing runs at all.

Two failures follow, both reachable today. A save still in flight when a
reset lands finds no row to conflict with, inserts the wiped page back,
and every ref on it goes on resolving. And a computer that restarts counts
generations from one again, so its first snapshot reads as older than the
dead page and is dropped until the counter climbs back past it.

That process serves every Bot and outlives every reset, so nothing outside
it can tell one browser session from the next. The session mints a run,
the reset handler mints a new one, and a read on the computer answers
which run a Bot is on. The sessions move to their own module because the
rules about when a run changes are worth testing, and index.ts launches a
browser the moment it is imported.

The run is also read before the snapshot rather than after it. Asked
afterwards, a computer replaced mid-snapshot would have its dead page
stamped with the run of the browser that replaced it, which is the same
pair of failures inside a smaller window.
The run changing on reset was covered by a unit test of the function that
changes it, which stays green if the handler never calls it, and that call
is the whole of the reset half. So the reset test now drives the real
process: the module is imported with a disposable profiles root and talks
over the port it opens, which works because neither of the endpoints it
uses ever asks for a page.

The shared-computer cases ran against the in-memory store, and a
deployment runs the other one. Ordering across runs is a comparison in
TypeScript in one and a setWhere Postgres evaluates in the other, so the
same halves now run against a real table, through the real provider, read
back through a second store the way another replica would.

Both files include the case where the computer cannot say which run it is
on, which has to leave every ref resolving rather than refuse it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant